-
Notifications
You must be signed in to change notification settings - Fork 1
fix the enable Minting button
#75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Deploying wildcat-dashboard with
|
| Latest commit: |
edd936c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://acd73319.wildcat-dashboard.pages.dev |
| Branch Preview URL: | https://peanut-minting-2.wildcat-dashboard.pages.dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes the "Enable Minting" button that was incorrectly disabled. The fix introduces a new minting_status field in the InfoReply type to properly track whether minting is enabled or disabled, replacing the previous logic that inferred minting status from keyset data.
Key changes:
- Added
MintingStatustype andminting_statusfield to the "Accepted" variant ofInfoReply - Renamed
newKeysetprop tomintingEnabledin theQuoteActionscomponent for clarity - Updated button disabled logic to use the new
minting_statusfield directly from the API response
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/generated/client/types.gen.ts |
Added MintingStatus type definition and minting_status field to InfoReply type for "Accepted" status variant |
src/pages/quotes/QuotePage.tsx |
Renamed prop from newKeyset to mintingEnabled, updated button logic to use new API field, removed obsolete badge display, and simplified "Request to Pay" button conditional |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
61e3f59 to
32b124b
Compare
src/pages/quotes/QuotePage.tsx
Outdated
| variant={newKeyset ? "default" : "destructive"} | ||
| className={newKeyset ? "bg-red-500" : "bg-blue-500"} | ||
| variant={mintingEnabled ? "default" : "destructive"} | ||
| className={minting ? "bg-red-500" : "bg-blue-500"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this say mintingEnabled?
zupzup
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
| className={minting ? "bg-red-500" : "bg-blue-500"} | ||
| > | ||
| {newKeyset ? "Disabled" : "Enabled"} | ||
| {mintingEnabled ? "Disabled" : "Enabled"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this button is disabled when minting is enabled? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
when minting has been enabled already, the button to enable it should be disabled
😅
it was mistakenly greied out all the times, now we have a new field in InfoReply that informs us on the status of the minting.
32b124b to
edd936c
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
📝 Description
it was mistakenly greied out all the times, now we have a new field in InfoReply that informs us on the status of the minting.